Skip to content

Add pydbus stubs #13921

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open

Add pydbus stubs #13921

wants to merge 22 commits into from

Conversation

Tatsh
Copy link
Contributor

@Tatsh Tatsh commented May 2, 2025

https://github.com/LEW21/pydbus/tree/master/pydbus

Upstream is extremely sporadic and currently not active.

In bus.pyi there are overloads for org.bluez and org.freedesktop.Notifications.

@Tatsh Tatsh force-pushed the pydbus branch 4 times, most recently from a4baf4d to d4870ab Compare May 2, 2025 05:15

This comment has been minimized.

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, not a full review, but two things I noticed:

You are using Any in many places, where a proper type could be substituted. In those cases, either remove the type annotation or use Incomplete from _typeshed where removing it would not work properly (for example, because Any is used in a union). In all remaining cases, Any needs to documented with the accepted or possible types and the reason for using Any.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@Tatsh Tatsh force-pushed the pydbus branch 2 times, most recently from 50cc2d0 to 1b118a4 Compare May 6, 2025 01:20

This comment has been minimized.

@Tatsh Tatsh requested a review from srittau May 6, 2025 01:44
srittau
srittau previously requested changes May 6, 2025
Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks mostly good, a few remarks below.

@Tatsh Tatsh requested a review from srittau May 6, 2025 15:17

This comment has been minimized.

@srittau
Copy link
Collaborator

srittau commented May 6, 2025

LGTM, but we need to find out why CI fails. I think it's because PyGObject-stubs doesn't declare a dependency on pygobject. Until someone beats me to it, I will investigate more tomorrow. You could try to add a stubtest_requirements in the [tool.stubtest] table of METADATA.toml for now, which should at least fix the stubtest error, but probably not the stub_uploader test.

This comment has been minimized.

@srittau srittau dismissed their stale review May 7, 2025 10:40

Outdated.

@srittau
Copy link
Collaborator

srittau commented May 7, 2025

@Tatsh Ok, I got stubtest running properly by adding some APT dependencies. But now there are a few proper stubtest complaints. Could you have a look?

This comment has been minimized.

This comment has been minimized.

@Tatsh
Copy link
Contributor Author

Tatsh commented May 7, 2025

I would prefer to not export Variant because I think it is a mistaken addition on the author's part.

This comment has been minimized.

class ProxyMixin(Generic[_T]):
def get(self, bus_name: str, object_path: str | None = None, *, timeout: int | None = None) -> _CompositeObject[_T]: ...
@type_check_only
class OrgBluezAdapter1Dict(TypedDict, total=False):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry I didn't notice this earlier, but type check only types should be prefixed with _, e.g. _OrgBluezAdapter1Dict.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to add # noqa: Y049 for these types. Ruff does not see these as used even though they are getting passed into TypedDict().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add it to the stubtest allowlist

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

github-actions bot commented May 7, 2025

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants